Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Index is strftime formatted, fixes #454 #512

Closed
wants to merge 3 commits into from
Closed

Index is strftime formatted, fixes #454 #512

wants to merge 3 commits into from

Conversation

onorua
Copy link
Contributor

@onorua onorua commented Feb 18, 2018 •
edited
Loading

fluentd does support index time templating, fluent bit - does not. This PR should fix that.

@onorua onorua mentioned this pull request Feb 26, 2018
Closed
edsiper
edsiper requested changes May 18, 2018
View reviewed changes
plugins/out_es/es.c Outdated
index_len = snprintf(j_index,
ES_BULK_HEADER,
ES_BULK_INDEX_FMT,
ctx->index, ctx->type);
es_index, ctx->type);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this ok when ctx->current_time_index == FLB_FALSE ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@onorua Did you address this question?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind. Looks not relevant in the latest.

@edsiper edsiper added the waiting-for-user Waiting for more information, tests or requested changes label May 18, 2018
@edsiper
Copy link
Member

edsiper commented Jul 20, 2018

ping

@onorua
Copy link
Contributor Author

onorua commented Jul 26, 2018

I have updated the PR, just for you to know

@StevenACoffman
Copy link
Contributor

StevenACoffman commented Aug 8, 2018 •
edited
Loading

@onorua I think you need to amend your commit with the git commit --amend -s; git push -f for the DCO check to pass.

Also, not sure why the tests are failing with this error:

[ 82%] Linking C executable ../bin/fluent-bit
../library/libflb-plugin-out_es.a(es_conf.c.o): In function `flb_es_conf_create':
/home/travis/build/fluent/fluent-bit/plugins/out_es/es_conf.c:253: undefined reference to `bool_value'
collect2: error: ld returned 1 exit status
make[2]: *** [bin/fluent-bit] Error 1
make[1]: *** [src/CMakeFiles/fluent-bit-bin.dir/all] Error 2
make: *** [all] Error 2

Your change seems not to touch this, and the failing code block in your branch isn't the same in master, so perhaps rebasing will fix it?

@onorua onorua force-pushed the 454-strftime-for-index-name branch 2 times, most recently from 8c21fc3 to e3bf5b2 Compare August 13, 2018 06:31
@onorua onorua force-pushed the 454-strftime-for-index-name branch from e3bf5b2 to f80ea98 Compare August 13, 2018 06:35
@edsiper
Copy link
Member

edsiper commented Sep 7, 2018

@StevenACoffman have you tried the changes on this PR?

@edsiper
Copy link
Member

edsiper commented Nov 23, 2018

@onorua in order to close/merge this one, would you please rebase your changes on top of GIT master ?

@onorua
Copy link
Contributor Author

onorua commented Nov 24, 2018

Sure, will do on Monday.

@onorua
Copy link
Contributor Author

onorua commented Nov 26, 2018

done

edsiper pushed a commit that referenced this pull request Feb 18, 2019
@onorua @edsiper
out_es: add current_time_index option (PR #512, #454)
279123a 
The following patch introduce a new boolean configuration option
called 'current_time_index'. When enabled all records and index
will take the current time instead of record time.

Signed-off-by: Eduardo Silva <[email protected]>
edsiper pushed a commit that referenced this pull request Feb 18, 2019
@onorua @edsiper
out_es: add current_time_index option (PR #512, #454)
455eae0 
The following patch introduce a new boolean configuration option
called 'current_time_index'. When enabled all records and index
will take the current time instead of record time.

Signed-off-by: Eduardo Silva <[email protected]>
@edsiper edsiper added the fixed label Feb 18, 2019
@edsiper
Copy link
Member

edsiper commented Feb 18, 2019

PR merged as a separate commit with required changes: 455eae0

@edsiper edsiper closed this Feb 18, 2019
edsiper pushed a commit that referenced this pull request Mar 20, 2019
@onorua @edsiper
out_es: add current_time_index option (PR #512, #454)
29b331b 
The following patch introduce a new boolean configuration option
called 'current_time_index'. When enabled all records and index
will take the current time instead of record time.

Signed-off-by: Eduardo Silva <[email protected]>
rawahars pushed a commit to rawahars/fluent-bit that referenced this pull request Oct 24, 2022
@Madhu-Kashyap
Update key-concepts.md (fluent#512)
c944509 
Fix minor grammatical error.

Signed-off-by: Madhu Kashyap <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StevenACoffman StevenACoffman StevenACoffman left review comments

@edsiper edsiper edsiper approved these changes

Assignees
No one assigned
Labels
fixed waiting-for-user Waiting for more information, tests or requested changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@onorua @edsiper @StevenACoffman